arch: riscv: Add function to clear all unlocked PMP entries #96185
Conversation
zephyrbot
requested review from
edersondisouza,
fkokosinski,
katsuster,
kgugala,
masz-nordic,
mgielda,
npitre,
tgorochowik,
VynDragon and
ycsin
|
According to the The RISC-V Instruction Set Manual: Volume II: Privileged Architecture CSR is accessed via csrr/csrw ... instructions, not load/store instructions, and the permission depend on the current privilege level, not on mstatus.MPRV or mstatus.MPP. I think this setup is not related to permission for access PMP CSR. Is this setup needed? |
fsammoura1980
force-pushed
the
reset_init
branch
from
6164968 to
83efc1f
Compare
I updated the commit message and comment. The main purpose is to put the mstatus MPRV and MPP bits into default state at the beginning of the init function. This is necessary when you have firmwares that jump from RO to RW portions, where the mstatus bits might have been altered during afirst firmware run. |
fsammoura1980
force-pushed
the
reset_init
branch
from
83efc1f to
f762d7f
Compare
| void z_riscv_pmp_init(void) | ||
| { | ||
| /* | ||
| * Ensure we are in M-mode and that memory accesses use M-mode privileges | ||
| * (MPRV=0) before configuring PMP registers. This prevents memory accesses | ||
| * during PMP setup from being restricted by a lingering MPP state. | ||
| * We also set MPP to M-mode to establish a predictable prior privilege level. | ||
| */ | ||
| csr_clear(mstatus, MSTATUS_MPRV); | ||
| csr_set(mstatus, MSTATUS_MPP); | ||
|
|
There was a problem hiding this comment.
Isn’t it too late to clean up mstatus and PMP CSRs in z_riscv_pmp_init()? Since z_riscv_pmp_init() is not the first function that accesses memory.
To ensure memory access use M-mode privileged, mstatus and PMP CSRs should be clean up in SoC low level initialization (before any load store instruction).
Or clean up mstatus, PMP CSRs before jump to this boot stage?
fsammoura1980
force-pushed
the
reset_init
branch
from
12426b6 to
885dd0b
Compare
fsammoura1980
changed the title
fsammoura1980
force-pushed
the
reset_init
branch
from
885dd0b to
6d99870
Compare
Introduce the new function `riscv_pmp_clear_all()` to securely reset the Physical Memory Protection (PMP) configuration. This function iterates through all configured PMP slots. For each entry, it checks if the Lock (L) bit is set. If the entry is not locked, it clears the Address Matching Mode (A) bits, effectively setting the region type to OFF (Null Region), disabling the entry. The function ensures it operates in Machine mode with MSTATUS.MPRV = 0 before modifying any PMP Control and Status Registers (CSRs). This provides a mechanism to clear all non-locked PMP regions, returning them to a default disabled state. The function is exposed in the pmp.h header file. Signed-off-by: Firas Sammoura <[email protected]>
fsammoura1980
force-pushed
the
reset_init
branch
from
6d99870 to
6db1573
Compare
|
Introduce the new function
riscv_pmp_clear_all()to securely reset the Physical Memory Protection (PMP) configuration.This function iterates through all configured PMP slots. For each entry, it checks if the Lock (L) bit is set. If the entry is not locked, it clears the Address Matching Mode (A) bits, effectively setting the region type to OFF (Null Region), disabling the entry.
The function ensures it operates in default Machine Status Registers (CSRs) with
MSTATUS.MPRV = 0andMSTATUS.MPP = 11.This provides a mechanism to clear all non-locked PMP regions, returning them to a default disabled state. The function is exposed in the pmp.h header file.